Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add segmentStyle prop to SegmentedControl #2563

Merged
merged 3 commits into from
Apr 19, 2023

Conversation

lidord-wix
Copy link
Contributor

Description

SegmentedControl - add segmentStyle prop (and update example)
#2495

Changelog

SegmentedControl - add segmentStyle prop

@@ -164,6 +169,7 @@ const SegmentedControl = (props: SegmentedControlProps) => {
selectedIndex={animatedSelectedIndex}
activeColor={activeColor}
inactiveColor={inactiveColor}
segmentStyle={segmentStyle}
Copy link
Collaborator

@Inbal-Tish Inbal-Tish Apr 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's an individual style for each segment, why it's not passed in the segment, meaning part of the SegmentedControlItemProps and not the SegmentedControlProps? It's confusing, or maybe it's just the naming...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it is confusing because I actually wanted one style prop for all of them. 😅
Maybe segmentsStyle in plural?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I guess segmentsStyle is better...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done :)

@lidord-wix lidord-wix requested a review from Inbal-Tish April 18, 2023 11:59
@@ -98,6 +102,7 @@ const SegmentedControl = (props: SegmentedControlProps) => {
outlineColor = activeColor,
outlineWidth = BORDER_WIDTH,
throttleTime = 0,
segmentsStyle: segmentsStyleProp,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need the rename here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's another segmentsStyle in this file

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok...

@lidord-wix lidord-wix requested a review from Inbal-Tish April 19, 2023 07:52
@Inbal-Tish Inbal-Tish merged commit 2cb96c2 into master Apr 19, 2023
lidord-wix added a commit that referenced this pull request May 15, 2023
* Add segmentStyle prop to SegmentedControl

* renaming

* renaming #2
@lidord-wix lidord-wix deleted the feat/SegmentedControl_segmentStyle branch May 29, 2023 14:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants